Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Local Jupyter Code Executor for Version 4 #4795

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Leon0402
Copy link
Contributor

@Leon0402 Leon0402 commented Dec 23, 2024

Why are these changes needed?

The new v4 version has no support for jupter notebooks yet compared to the old version.

NOTE: This is still a draft and largely untested. But some early feedback is appreciated :)

Related issue number

Closes #4792

Checks

@Leon0402 Leon0402 marked this pull request as draft December 23, 2024 14:47
@Leon0402
Copy link
Contributor Author

@microsoft-github-policy-service agree

@jackgerrits jackgerrits added this to the 0.4.1 milestone Dec 23, 2024
@Leon0402 Leon0402 marked this pull request as ready for review December 23, 2024 22:20
@Leon0402
Copy link
Contributor Author

@jackgerrits @ekzhu PR is now ready for review.

  • I executed tests locally and some tests are still failing for the command line executor, but that seems unrelated to my changes as far as I can tell
  • I have one open design question (see above in the comment) which should be fixed before merging
  • I did not write any documentation. If it is okay I would leave this up to you. You know best whether to merge this in with the existing documentation of command line executor or create a new page combining all the information (as done previously)

@ekzhu
Copy link
Collaborator

ekzhu commented Dec 24, 2024

@Leon0402 thanks for the PR. There is a possibility that we may get to this after the 0.4.0 release. Please let us know if this is something you need urgently.

@Leon0402
Copy link
Contributor Author

@Leon0402 thanks for the PR. There is a possibility that we may get to this after the 0.4.0 release. Please let us know if this is something you need urgently.

Yes, it is something I need quite urgently. But I am okay with working with my fork until it is released and I obviously understand that your availability is limited over the Christmas period.

@Leon0402 Leon0402 force-pushed the feature/jupyterCodeExecutor branch 2 times, most recently from f53e14a to 4ac263d Compare December 24, 2024 09:23
@Leon0402
Copy link
Contributor Author

Redesigned a little bit and added more tests, should be in a good state now.

@ekzhu One thing I wonder is how autogen previously dealt with images. I currently modified the code such that the result is that output contains something like this "<PIL.Image.Image image mode=RGB size=100x100>" and there is an array of file path containing the path to the actual image. Previously the output also contained something like "Image saved to path ...", but I felt that this was always redundant (in my applications I filtered that usually).

As far as I understood my application code (not written originally by me) in Autogen V2 it would postprocess the output to:

  1. Remove this "Image saved to path ..." (as explained above)
  2. Replace the Pil Image by an html image tag with source attribute set to the file path

Then https://github.com/microsoft/autogen/blob/v0.2.40/autogen/agentchat/contrib/img_utils.py#L166 would parse this automatically such that the image is included in the prompt / chat history.

I wonder if we should adjust the code here that it produces image tags directly? Possibly even base64 encoded directly. Because right now all images are saved in the working directory. I don't think this is really necessary.

Thus my proposal is to store image tags with base64 uri directly. Any thoughts?

@Leon0402 Leon0402 force-pushed the feature/jupyterCodeExecutor branch from 96bb175 to 661d670 Compare December 28, 2024 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Port Jupyter code executor from v0.2
3 participants